Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Require MFA on the top 100 most downloaded gems #36

Merged
merged 9 commits into from
Mar 18, 2022

Conversation

jenshenny
Copy link
Member

Here’s the RFC that was mentioned in this issue on a policy to require MFA on a cohort of gems.

Contributors: @bettymakes, @aellispierce, @jchestershopify, @jenshenny

We’re all available on the Bundler Slack to discuss this also!

@jenshenny
Copy link
Member Author

Even though Shopify will be able to commit a certain number of developers to work on this policy if it's accepted, we acknowledge that there is time that needs to be spent by the Rubygems maintainers to oversee the contributions and policy changes. We would be happy to discuss ways to reduce this time and workload as much as possible.

Co-authored-by: Ashley Ellis Pierce <anellis12@gmail.com>
Co-authored-by: bettymakes <makewithbetty@gmail.com>
Co-authored-by: Jacques Chester <jacques.chester@shopify.com>
@jenshenny jenshenny changed the title RFC: Require MFA on top 100 most downloaded gems RFC: Require MFA on the top 100 most downloaded gems Jan 12, 2022
text/0000-mfa-rollout.md Outdated Show resolved Hide resolved
text/0000-mfa-rollout.md Outdated Show resolved Hide resolved
@simi
Copy link
Member

simi commented Jan 13, 2022

I went just briefly thru the RFC and I must say it is really nicely written and it makes total sense. The only problem I see is scope of the top 100 most downloaded gems. That is rolling scope. Gems can go in and out over the time.

Instead, I think we can make the threshold based on some metric and only add new gems to the scope. For example we can find threshold of total downloads for current top 100 gems and make it the rule.

When total amount of downloads of gem reaches XY, we will enforce MFA to publish new release. We can also ping the authors not using MFA in advance when 90% (or any reasonable part) of threshold is reached.

@mensfeld
Copy link
Member

@simi I though about the same. Since it's a supply chain issue, if we decide to "2FA lock" a package, its dependencies should be locked as well. Otherwise we leave weak links in the chain for potential exploitation.

I am aware, that the dependency states change across the versions, so I would suggest locking the mostly used versions (if we have those metrics) based on one last year + any newly introduced.

This may also require monitoring of changes introduces to the dependency tree of "2fa locked" packages. I am willing to help with that as I have extensive experience in mining this data.

@rst
Copy link

rst commented Jan 13, 2022

Agree that 2fa-locking dependencies of the "top 100" makes the policy a whole lot more effective. But that puts restrictions on the maintainers of those top 100 packages which could get awkward for them. Most notably, they'd have to be barred from adding a new dependency until its maintainer got 2fa. It's probably worth thinking through the workflow here, for all parties, and making sure it lands in an acceptable place.

@jchestershopify
Copy link
Contributor

Since it's a supply chain issue, if we decide to "2FA lock" a package, its dependencies should be locked as well. Otherwise we leave weak links in the chain for potential exploitation.

We do, yes.

The goal here is really to get the ball rolling. Top 100 most-downloaded is something of an arbitrary selection, so if we have a better heuristic (eg download thresholds as suggested by @simi), that would be fine too. So long as we start, stabilise and then continue to make steady progress towards high MFA coverage.

@tiegz
Copy link

tiegz commented Jan 13, 2022

Love this!

Do we know how effective locking access to Rubygems will be as an enforcer here? e.g. how many packages in the top 100 aren't maintained regularly, which would prevent the maintainer from being aware of the lockout?

Any thoughts on publicly exposing the MFA status on the gem page (or maybe the rubygems_mfa_required status since it's already public?)? Obviously it makes the gem more of a target and I'm not aware of another platform that does this, but it could also prompt consumers of gems to urge the maintainers to enable it.

@tiegz
Copy link

tiegz commented Jan 13, 2022

(or maybe the rubygems_mfa_required status since it's already public?)?

Woops, nevermind, just noticed this is already public! 👍🏻

aellispierce and others added 2 commits January 13, 2022 11:16
The framing of timelines as seasons are not region agnostic (northern vs southern hemisphere).
Reframing around calendar quarters enables us to keep some flexibility in timeline commitments.
@jchestershopify
Copy link
Contributor

A nitpick that I just noticed: the filename should probably start with 0007- instead of 0000-.

@mullermp
Copy link

Hey there, I've updated the the awscloud account to have MFA with the "UI and gem signin" level. Using MFA for API is a bit tricky because of CI/CD and would require larger effort. Also, keys scoped to each gem might be infeasible for us because 1) we maintain many gems, 2) we often need to access all of them at the same time, and 3) add generate, store, and retrieve new ones for each service launch.

@hsbt - I got your request from Alex Wood who no longer works on AWS SDK For Ruby. Feel free to create an issue on the main repo if you need us :D

@jenshenny
Copy link
Member Author

The CI/CD case is definitely tricky. Enforcing keys to be scoped to a gem on a non-MFA API key might be too restrictive as you said @mullermp, and a better alternative would need to be thought out. The Shopify Rubygem account is in a similar situation. With that being said, requiring users to have the UI and gem signin level is a good first step for this path.

@jenshenny
Copy link
Member Author

The only problem I see is scope of the top 100 most downloaded gems. That is rolling scope. Gems can go in and out over the time.

Instead, I think we can make the threshold based on some metric and only add new gems to the scope. For example we can find threshold of total downloads for current top 100 gems and make it the rule.

To come back to this @simi, in the recommended prototype, to address the issue of gems going in and out of scope is to run a task to set a date for mfa enforcement for each user that fits in the metric. This task could be ran periodically to capture new gems entering the scope. According to the proposal, we selected a 6 mo period for the top 100 most downloaded gem accounts (can be changed). Would that approach work well?

Based on your suggestion of finding a threshold of total downloads for current top 100 gems and make it the rule, I assume the alternative would be to check if it reaches this threshold when a gem download is incremented?

When total amount of downloads of gem reaches XY, we will enforce MFA to publish new release. We can also ping the authors not using MFA in advance when 90% (or any reasonable part) of threshold is reached.

From the point of when we set the date on enforcement on a user, they will get messages on the CLI and UI to set up MFA until the required date is reached. Do you think it's just sufficient enough to just ping the user (I'm guessing via email) instead of these prompts?

Let me know if you want me to add a description of the technical approach in the md instead of having them in the linked prototype PR descriptions.

@simi
Copy link
Member

simi commented Jan 29, 2022

What to do when user rejects to use MFA? Does it mean user is going to lose access to releasing gems?

What about to keep the possibility to release gem, but mark the gem as "unsafe" and let the end-user know during installation this gem was not released in secured way? That way we don't need to enforce users, but we will inform end-users, which can go and ask maintainer to release in secure way. Clearly there will be warning during gem push (we can also make it fail in later phase without explicitly passing --insecure flag to gem push for example).

By my experience, enforcing MFA (or actually enforcing anything in general) is painful process for users. Making it optional, but little more "painful" when not used seems like better option to me.

text/0000-mfa-rollout.md Outdated Show resolved Hide resolved
text/0000-mfa-rollout.md Outdated Show resolved Hide resolved
@jenshenny
Copy link
Member Author

What to do when user rejects to use MFA? Does it mean user is going to lose access to releasing gems?

In the current state, yes. You raise a good point that users can’t release gems especially in an emergency situation.

At this moment, I’m open to the possibility of marking unsafe on gems. This is something we can do for the entire rubygems population vs a cohort (it would be confusing if 1. a gem without mfa in a subset is marked unsafe, vs 2. others that aren’t in the subset, doesn’t have mfa but isn’t marked unsafe).

However, from this discussion, I’m not sure if this will encourage shaming if we want to inform end users during installation especially for a highly downloaded gem, though I guess it might be warranted if prior notice was given. It could do more harm than good and prolong adoption. I’m leaning towards using this solution for the general population and catch gem dependencies to enable MFA, but still require MFA on the most frequently used gem user accounts if adoption isn’t high enough before marking things unsafe.

@jchestershopify
Copy link
Contributor

jchestershopify commented Jan 31, 2022

I agree with @jenshenny -- I don't think a warning and shaming will significantly move the needle. Most end users are busy and almost all of them are CI/CD robots who don't read warnings.

What I'd like to do is reframe how we think about security issues like MFA. I think end users should be at the centre of every decision and their needs should be put first.

For example, consider an account takeover. For a gem maintainer, an account takeover is distressing and embarrassing. But for end users, it is potentially catastrophic. And end users outnumber maintainers by a large ratio. In terms of a calculus of harm, I think end users of gems deserve to be given priority. That means that they need maintainer accounts to be secured with MFA.

@jenshenny
Copy link
Member Author

jenshenny commented Feb 8, 2022

NPM announced that they are requiring MFA to the top 100 npm package maintainers that started last week, and rolling it in phases to all accounts by March. I believe Rubygems should also follow NPM's lead and follow a similar policy and rollout. If a maintainer doesn't want to enable MFA, we can make it a bit more "annoying" by making them enter an OTP sent to their email on the UI and gem signin.

@schneems
Copy link

My experience

I've been using MFA on all my gems https://rubygems.org/profiles/schneems for several years now. My employer, Heroku, required it. I think it's a nice layer of protection and while it does add another step to releasing, it's (to me) a small one.

I release via rake release with the bundler release tasks. It prompts me for a MFA code. I've got that set up on my laptop already so I open an app, put in a passcode, type in "ruby" copy and paste the MFA in and then I'm done. I could probably automate it more to require fewer steps, but I've not (personally) felt the need.

My opinion

It's my opinion/experience that MFA is a large bang-for-your-buck and is already expected in many/most places.

I would like us to be "secure by default" as much as possible.

There are some implementation questions around how to get there. If someone "rejects" the requirement to use MFA they can run their own RubyGems server to host their own gems and release in that way. If a gem says "add this other source to your Gemfile" that's at least a traceable artifact where someone could visually audit the Gemfile and identify all secondary sources that might not be as secure.

I would support a rolling process for making all gems MFA as it only takes one broken link to have a supply chain attack. Starting at the first 100 most popular is a good step, but doesn't go far enough (again, my opinion). If there's an escape valve of "run your own gemserver if you really hate it that much" I think that handles edge cases. I do think it's a good idea to think about the edge cases, but as long as there is at least one escape valve that covers them, I don't know that we need to overly go out of our way to try to please everyone.

I think with security stuff it's a balance between things that reasonably reduce attack surface area versus usability. One example of this going wrong might be a company that aggressively enforces password rotation frequently and then employees start writing their password down by their desk because they don't have enough time to memorize it. It's important to think on possible unintended consequences, but we also will not come to a case of 100% consensus of all cases perfectly handled.


# Unresolved questions

- Legacy keys are a relic of pre Rubygems 3.2 but still are in existence. What would the impact be of removing legacy key support (it comes with removing Rubygems <3.2 also)? Legacy keys bring a larger risk to accounts and can bypass the restrictions we are placing on API keys. More specifically, how many legacy keys are still being used recently among the 100 most downloaded gem accounts and the greater ecosystem? Are there any metrics determining the amount of people in Rubygems <3.2?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legacy keys are not in use, at least from the perspective of rubygems.org. All legacy keys were migrated to the new API key and will support any limitation added on MFA. The GET endpoint used in clients older than 3.2 also creates the new API key.

- Changed the metric from top 100 most downloaded gems to a download threshold of 175 million
- Updated the timelines to enforce the policy in July/August
- Add more specific communication commitments (sending emails to affected users, writing blog posts)
Copy link
Member Author

@jenshenny jenshenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the RFC so that we use a download threshold to determine which gems should require MFA. sinatra has ~179 million downloads, so a reasonable threshold would be 175 million. Using today's data dump, this would include 2 more gems with a maximum of 6 additional affected users.

I also updated the timelines to enforce the policy on July/August and added a note about sending emails and writing a blog post.

Gem publishing will be improved by allowing gem owners to push mulitple gems at once, so that they only need to enter their OTP code one time. A prototype for pushing multiple gems has been briefly explored here: https://github.com/Shopify/rubygems/pull/2

## Policy Rollout
A user account will be required to have MFA enabled if they own a gem that exceeds a download threshold of 175 million. This number was selected as it is around the total downloads of the 100th most downloaded gem ([sinatra](https://rubygems.org/gems/sinatra)). There will be two methods introduced in the Rubygems model. One would determine whether the gem owners are encouraged to setup MFA (eg. `mfa_recommended?`) and display relevant warnings on the UI/CLI. The other would determine if gem owners are required to setup MFA (eg. `mfa_required?`) and start blocking sensitive behaviours.
Copy link
Member Author

@jenshenny jenshenny Feb 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also ping the authors not using MFA in advance when 90% (or any reasonable part) of threshold is reached.

I don't have much context around the rate of downloads but would it make more sense to warn/ping users ASAP at around ~170 million, and enforce at ~175 million in July/Aug? This would prevent owners on the border to be surprised once they reach the required threshold.

Copy link
Contributor

@bettymakes bettymakes Mar 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of introducing a signal earlier before gem owners have reached the 175 million threshold 👍. If this does get implemented, I'd recommend warning them sooner than ~170 million downloads since by that point they'll be ~97% away from reaching the ~175 million threshold.

The guidance of ~90% away from threshold seems reasonable. That would translate to warning at ~157 million downloads, or round up to ~160 million if round numbers are preferred.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 That makes sense, I updated the thresholds to 180 million for required since sinatra (100th most downloaded gem) has reached that mark and 165 million for recommended. 165 million is roughly 90% of 180 million.

@jenshenny
Copy link
Member Author

@simi @sonalkr132 Bumping this up to see if you have any feedback on the above changes.

@simi
Copy link
Member

simi commented Mar 6, 2022

Bumping this up to see if you have any feedback on the above changes.

Thanks for the ping @jenshenny.

I'm frustrated to enforce something for my OSS work from a big tech company's order. But I agreed to this proposal.

I do hear you @hsbt. I'm thinking how to prevent this kind of feeling for other gem owners as well. If I understand it well, nothing is being enforced for now, right? We need to ensure that "enforcing" part would be smooth as possible to not make anyone feeling this way.

I think we basically all agree on phase 1 and 2. I was thinking how to move this forward. What about to split this RFC into two parts? Preparation + recommended part (phase 1 and 2) and "enforcing" part (phase 3 and 4). That way I think we can consider first part as agreed and merge this part of RFC. We can still do some small tweaks in there, but the overall goal would stay the same.

And for the second part I suggest to open separated RFC (we can scope this one only to first part) and we can continue in there. I think we can keep the dates open unless we find smooth solutionfor enforcing.

WDYT @jenshenny @hsbt @sonalkr132 @jchestershopify @tiegz @schneems @mensfeld @rst?

@sonalkr132
Copy link
Member

I am unsure about part 4 as well. We don't need to enfore mfa on gems if they are just personal projects. Anything below 20k downloads would fall this category for me (includes 83% of gems).
I think we can go ahead with part 3 altough it doesn't mean much without same check on dependencies. Perhaps we should check if we were enfore this only top 100 and all their transitive runtime dependencies, how many gems/users will it affect?
Asking to enable to MFA for user:password login seems like an acceptable ask to me, given that it won't affect CI pipelines and we already have decent adoption in top 100.

@mensfeld
Copy link
Member

mensfeld commented Mar 7, 2022

@simi I agree with the enforcement but I also see @hsbt point of view though I disagree with it. I think that 2fa in the case of popular packages should just become a standard.

I think improvements to the gem publishing flow as well as recommendations should be implemented as soon as doable for the benefit of the ecosystem. So phases 1 and 2 are 👍 from me for sure.

@sonalkr132

Anything below 20k downloads would fall this category for me (includes 83% of gems).

Then we have a case of new dependencies coming to the supply chain while not being popular (yet). For example, when faraday-rack was published not long ago (last year) it immediately became transitive dependency for many projects while still being (for a short period of time) < 20k. Projects are merged and splitted all the time and download count may not be the best metric for new packages being introduced into the chain.

Maybe we should make it not based on downloads but rather based on a collective dependency tree popularity? That way we would protect the whole supply chain for popular components (collectively) while not introducing any burden to private projects.

@jenshenny
Copy link
Member Author

I hope we can also go ahead with phase 3. With its current adoption in the top 100, the small number of users to enforce this policy on, and the positive impact to the many consumers using these gems, it's a fair ask for users to enable MFA for UI and gem signin.

I am unsure about part 4 as well.

Phase 4 was intended to be vague and convey that a rollout wouldn't stop at just the top 100. There was a gap in context to determine whether it made sense to make it required by default or if they had to reach certain requirements. I clarified in Phase 4 that the details of this would be discussed in another RFC. From the discussion here however, I imagine using a download threshold is easier than using a dependency tree. Perhaps we can go with a download threshold and later refine it to use a dependency tree if it's valuable to.

@jchestershopify
Copy link
Contributor

From the discussion here however, I imagine using a download threshold is easier than using a dependency tree.

That's my belief also. They'll be fairly well correlated in the long run, even if in the short run (such as faraday-rack) there could be outliers.

@mensfeld
Copy link
Member

mensfeld commented Mar 8, 2022

@jchestershopify It's not often that the chain is long. Calculating 1 or 2 previous links for the "forcing 2fa" would be trivial for new packages. For older downloads count would be enough. Even with the current setup of rubygems it would require at most 2 additional calls to the db (I may be wrong on it though) for this data and only in case of new versions of packages that are below threshold.

@sonalkr132
Copy link
Member

In my experience, dependency chain is not as easy to calculate. It would inevitably need recursion and we won't be able to add it for "on the fly" check. You also need to consider (all?) versions, for example:

  • A is one of top 100 gem
  • 1.0 of A requires ~> "1.0" of B
  • 1.1 (latest) of A does not require B
  • B has active development on 0.9.x and 1.0.x series

Do we inforce mfa requirement on B even if latest of A doesn't require B? Do we inforce mfa on 0.9.x release of B as well even if it is not used by A?
If we add an offline process to mark gems for mfa requirement, how do we ensure that we don't have flapping issue simi brought up.

- Recommended threshold to 165 million (reflects 90% of the required threshold)
- Required threshold to 180 million (reflects 100th most downloaded gem downloads)
@jenshenny
Copy link
Member Author

@simi you suggested to only merge phases 1 and 2. How do you feel about merging the RFC in its current state with part 3 included since there’s support in doing so in subsequent comments?

Are there any other blockers that are noticeable regarding merging the RFC in this current state (with phase 4 discussed in another RFC)?

Perhaps we should check if we were enfore this only top 100 and all their transitive runtime dependencies, how many gems/users will it affect?

Using this script (which I hope is correct) with today's data dump, the total number of dependencies it'll affect would be ~750. Only counting runtime dependencies, there’s 425 and the least downloaded gem is aws-keyspaces which was created a week ago. The one after has more than 850 000 downloads. If we continue this rollout on a rolling basis and stop at the download threshold for personal projects, I think it would work well.

@simi
Copy link
Member

simi commented Mar 17, 2022

@jenshenny thanks for moving this forward. RFC looks good to me now. Since phase 4 would be specified later in separate RFC and for phase 3 there is just estimate date (and also since nothing is set into stone even after we merge this) I'm ok to merge this.

To follow the process (https://github.com/rubygems/rfcs#what-the-process-is), should we call for final comment period? @indirect @sonalkr132 @deivid-rodriguez any idea?

@sonalkr132
Copy link
Member

We have not followed the process by the book until now. Generally we just merge if everything looks good. I would be okay with merging this as is.

@simi
Copy link
Member

simi commented Mar 18, 2022

OK, let's do it.

@simi simi merged commit 2fa25ba into rubygems:master Mar 18, 2022
@simi
Copy link
Member

simi commented Mar 18, 2022

🥳

@ioquatix
Copy link

ioquatix commented Mar 25, 2024

Eric, the maintainer of Unicorn, has said that it is hard to release updates to Unicorn as he is unable to use MFA (* this is my best interpretation of the current situation).

This might be a net security problem if maintainers can't release security updates to gems, especially popular ones. That being said, I don't know of any other maintainer who is in this position.

As a compromise, what about allowing some users to opt out of MFA requirements? i.e. opt in by default as is current behaviour, but allow explicit opt out. We can also document that clearly, e.g. "This gem has opted out of MFA security."

@simi
Copy link
Member

simi commented Mar 25, 2024

@ioquatix is there anything we can do to help release Unicorn? Is there any info why current MFA options are not usable for given usecase? I think it would be better to improve MFA and make it easier to adopt instead of adding additional feature to bypass the requirements.

@indirect
Copy link
Member

One easier to use type of MFA that we have not (yet?) implemented is: send a code in an email, request the code to be entered. Since RubyGems.org accounts already require an email and password, it seems like that should be usable by anyone who already has a RubyGems.org account. @ioquatix do you know if that would suffice?

@ioquatix
Copy link

We'd have to contact Eric to see what would work for him.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Require MFA
Done ✅
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet